refactor: cut all database call sites over to the facade#7407
Open
diegolmello wants to merge 5 commits into
Open
refactor: cut all database call sites over to the facade#7407diegolmello wants to merge 5 commits into
diegolmello wants to merge 5 commits into
Conversation
Introduce a temporary compatibility layer that exposes the WatermelonDB
public API (Database, Collection, Query, Model, decorators, Q clauses,
sanitizedRaw, appSchema/tableSchema) on top of the synchronous Drizzle
expo-sqlite driver. A fetched Drizzle row is column-keyed and therefore
structurally identical to WatermelonDB's _raw, so the facade Model wraps
the row directly and field getters read _raw[column].
This lets the ~80 existing @nozbe/watermelondb import sites be re-pointed
onto the facade without touching call-site logic, ahead of removing the
WatermelonDB package entirely in a later step.
Notable deviations from WatermelonDB, required by Drizzle:
- sanitizedRaw emits { id, ...coercedColumns } only — no _status/_changed,
which have no Drizzle columns; a random 16-char id is generated when absent.
- write/batch wrap the synchronous Drizzle transaction in a Promise surface;
a single-writer WriterQueue serializes writers.
- observe()/observeWithColumns() bridge expo-sqlite's change listener to RxJS,
filtering by database file + table, debouncing 16ms, and structurally
sharing unchanged row references.
Verified: tsc, eslint, and 32 unit tests pass on a fresh empty database.
Comparison operators (eq, notEq, gt, gte, lt, lte, like, notLike, oneOf) take
only the right-hand value and return a Comparison; Q.where(column, valueOrComparison)
wraps it, treating a raw value as an implicit eq. This matches every call site
(e.g. Q.where('ts', Q.gt(date)), Q.where('id', Q.oneOf(ids))) ahead of the cutover.
Null comparisons now lower to IS NULL / IS NOT NULL rather than = NULL, which is
never true in SQL — required for the many Q.where(col, null) and Q.notEq(null) sites.
Replace @nozbe/watermelondb imports across the app with the WatermelonDB-shaped facade over the Drizzle/expo-sqlite driver, so the app runs on the encrypted SQLCipher database while keeping call sites unchanged. Add the table/model maps the facade needs to construct each Database, wire the servers and app schemas through it, and add an ESLint rule banning new direct watermelondb imports. WatermelonDB stays installed only for the legacy migration reader; no JS runtime path uses it anymore.
10 tasks
Contributor
WalkthroughIntroduces a complete WatermelonDB-compatible database facade backed by Drizzle ORM and expo-sqlite, implemented as ChangesDatabase Facade Implementation and Migration
Sequence Diagram(s)sequenceDiagram
participant App
participant DB as DB (index.ts)
participant Database as Database (facade)
participant WriterQueue
participant Collection
participant DrizzleSQLite as expo-sqlite/Drizzle
App->>DB: initServers()
DB->>DrizzleSQLite: openServersDb()
DrizzleSQLite-->>DB: DbHandle
DB->>Database: new Database(handle, serversSchema, tableMaps)
DB-->>App: servers DB ready
App->>DB: setActiveDB(server)
DB->>DrizzleSQLite: openServerDb(server)
DrizzleSQLite-->>DB: DbHandle
DB->>Database: new Database(handle, appSchema, appTableMaps)
DB-->>App: active DB ready
App->>Database: write(fn)
Database->>WriterQueue: enqueue(fn)
WriterQueue->>fn: execute (serialized)
fn->>Database: batch(models)
Database->>DrizzleSQLite: transaction (INSERT/UPDATE/DELETE per _pendingOp)
DrizzleSQLite-->>Database: committed
Database-->>App: Promise<void>
App->>Collection: query(Q.where(...)).observe()
Collection->>DrizzleSQLite: addDatabaseChangeListener
DrizzleSQLite-->>Collection: change event (debounced)
Collection->>App: Observable<Model[]> emission
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/lib/database/driver/connection.ts (2)
176-201:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClose the SQLite handle when open/migration fails.
If
applyOpenPragmas()(Line 186) ormigrate()(Line 196) throws,sqliteremains open and uncached, leaking a live handle and potential file lock.💡 Proposed fix
async function openDb<K extends DbKind>(dbName: string, kind: K): Promise<DbHandle<K>> { const cached = _registry.get(dbName); if (cached) { return cached as DbHandle<K>; } const keyHex = await getOrCreateDatabaseKey(dbName); - - const sqlite = await openDatabaseAsync(dbName, { enableChangeListener: true }, DB_DIRECTORY); - - await applyOpenPragmas(sqlite, keyHex); - - const schema = kind === 'servers' ? serversSchema : appSchema; - // The conditional type SchemaForKind<K> cannot be narrowed by the JS runtime check above; - // casting through unknown is the standard TS pattern for this shape. - const db = drizzle(sqlite, { schema }) as unknown as ExpoSQLiteDatabase<SchemaForKind<K>>; - - // Apply the schema DDL on the freshly keyed handle. The migrator creates tables on first - // open and tracks applied migrations in __drizzle_migrations, so re-opens are no-ops. - const migrations = (kind === 'servers' ? serversMigrations : appMigrations) as MigrationConfig; - await migrate(db, migrations); - - const handle: DbHandle<K> = { db, sqlite, dbName }; - _registry.set(dbName, handle as DbHandle); - return handle; + let sqlite: SQLiteDatabase | undefined; + try { + sqlite = await openDatabaseAsync(dbName, { enableChangeListener: true }, DB_DIRECTORY); + await applyOpenPragmas(sqlite, keyHex); + + const schema = kind === 'servers' ? serversSchema : appSchema; + const db = drizzle(sqlite, { schema }) as unknown as ExpoSQLiteDatabase<SchemaForKind<K>>; + + const migrations = (kind === 'servers' ? serversMigrations : appMigrations) as MigrationConfig; + await migrate(db, migrations); + + const handle: DbHandle<K> = { db, sqlite, dbName }; + _registry.set(dbName, handle as DbHandle); + return handle; + } catch (e) { + if (sqlite) { + try { + await sqlite.closeAsync(); + } catch { + // ignore cleanup failure; preserve original error + } + } + throw e; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/database/driver/connection.ts` around lines 176 - 201, The issue in the openDb function is that if either applyOpenPragmas or migrate throws an error, the sqlite handle opened by openDatabaseAsync remains unclosed and uncached, causing a resource leak. Wrap the applyOpenPragmas and migrate operations in a try-catch block, and in the catch block, close the sqlite handle by calling its appropriate close method before re-throwing the error to ensure proper cleanup.
71-85:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
deriveServerDbName()can collide with the global servers DB filename.Line 71 reserves
default.dbforopenServersDb(), and Line 84 can generate that same value for app DBs (e.g., server URLhttps://default). That can route app and servers traffic to the same file/registry key.Also applies to: 210-220
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/database/driver/connection.ts` around lines 71 - 85, The deriveServerDbName() function can generate the same filename as the reserved DEFAULT_DB_NAME constant ('default.db'), which occurs when processing server URLs like 'https://default'. This creates a collision where app database traffic and global servers database traffic get routed to the same file. Fix this by adding a collision detection and prevention mechanism in deriveServerDbName(): after deriving the sanitized database name, check if it matches DEFAULT_DB_NAME and if so, apply a differentiating suffix or alternative naming strategy to ensure uniqueness. Apply the same fix to the other similar logic mentioned at lines 210-220 to maintain consistency across all database filename derivation.
🧹 Nitpick comments (1)
app/lib/database/facade/decorators.ts (1)
119-125: ⚡ Quick winUse
_jsonDecoratorCachein@jsongetter to preserve memoized behavior.Line 119-125 reparses and re-sanitizes on every access, while
Modelalready allocates_jsonDecoratorCachefor this decorator path. This adds avoidable work and can break stable-reference expectations.Suggested patch
get(this: AnyModel): unknown { const model = this.asModel; const rawValue = model._getRaw(rawFieldName); + const cached = model._jsonDecoratorCache[rawFieldName]; + if (cached && cached[0] === rawValue) { + return cached[1]; + } const parsedValue = parseJSON(rawValue); const sanitized = sanitizer(parsedValue, model); + model._jsonDecoratorCache[rawFieldName] = [rawValue, sanitized]; return sanitized; },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/database/facade/decorators.ts` around lines 119 - 125, The `get` method in the `@json` decorator is re-parsing and re-sanitizing the value on every access instead of using the memoization cache. Modify the getter to first check if a cached value exists in the model's `_jsonDecoratorCache` using the `rawFieldName` as the key. If cached, return it immediately. Otherwise, perform the existing parsing and sanitization logic (parseJSON and sanitizer calls), store the result in `_jsonDecoratorCache` with the `rawFieldName` key, and then return the sanitized value. This preserves the memoized behavior and avoids redundant parsing and sanitization work on subsequent accesses.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/database/facade/Collection.ts`:
- Around line 61-66: The truthy check on filter?.id at line 61 incorrectly skips
the id filter when filter.id is a valid falsy value like 0, false, or an empty
string, causing the code to fall back to full-table selection. Replace the
truthy check if (filter?.id) with a presence check that distinguishes between
falsy values and undefined/null, such as checking if ('id' in filter) or using
filter?.id !== undefined, so that the where clause with eq(idCol, filter.id) is
applied regardless of whether the id value is falsy.
In `@app/lib/database/facade/Database.ts`:
- Around line 79-110: The issue is that model._pendingOp is being cleared inside
the transaction loop at line 109, which means if a subsequent database operation
fails and the transaction rolls back, the earlier models will already have
_pendingOp set to null, causing the retry logic to silently skip them. Move the
model._pendingOp = null assignment outside of the transaction callback function
so that it only executes after the entire transaction successfully completes,
ensuring that if any operation fails and the transaction rolls back, all models
still have their _pendingOp set and can be retried.
In `@app/lib/database/facade/decorators.ts`:
- Around line 242-249: The `observeRow` function used in the return statement
suppresses null values, but the declared return type `Observable<T | null>`
indicates the observable should emit null when relations are removed or deleted.
Replace the use of `observeRow` with an alternative observation mechanism that
preserves null emissions, ensuring that when the foreign key is cleared or the
related row is deleted, the observer receives a null value as expected by the
contract defined in the return type signature.
- Around line 173-183: The Collection constructor in the `@children` decorator is
being passed the generic Model class instead of the child table's specific model
class, which causes rows to lose child-specific decorators and getters. Extract
the actual model class from the childCollection (similar to how the table,
schema, _drizzleTable, and _handle are being extracted via type assertions) and
pass that model class as the final argument to the Collection constructor
instead of the generic Model parameter.
In `@app/lib/database/facade/observe.ts`:
- Around line 73-82: Wrap the fetchFn() calls in try-catch blocks throughout the
file to properly handle exceptions through RxJS error handling. In the emit
function (starting at line 73), wrap the const fresh = fetchFn() call in a
try-catch block, and in the catch clause, route the error to subscriber.error()
before returning. Apply the same pattern to the other two emit paths mentioned
(around lines 108-112 and 143-157) to ensure all fetchFn() exceptions are
properly caught and passed through subscriber.error() instead of propagating as
unhandled runtime exceptions when triggered by setTimeout callbacks.
- Around line 177-184: The sameByColumns function only compares watched column
values at matching array indices without checking row identity. If rows reorder
or a different row with identical watched column values replaces a row at the
same index, the function incorrectly returns true and suppresses necessary
update emissions. Add a row identity check before comparing column values in
sameByColumns to ensure that even if two rows have matching watched column
values, they are still treated as different if they have different ids. Compare
the row identities first (using the id property from HasId) to catch reordered
or replaced rows.
In `@app/lib/database/facade/writer.ts`:
- Around line 19-24: The enqueue method fails to handle synchronous exceptions
thrown by fn(). When fn() throws synchronously before returning a promise, the
second .then() handler with dual undefined callbacks catches the error and
resolves to undefined instead of rejecting the caller's promise, causing resolve
and reject to never be invoked. Wrap the fn() call in a try-catch block within
the first .then() callback to catch synchronous exceptions and call
reject(error) when they occur, ensuring the caller's promise always settles
regardless of whether fn() throws synchronously or asynchronously.
In `@app/lib/database/index.ts`:
- Around line 60-63: The `setActiveDB()` method has a race condition where
multiple rapid calls can result in out-of-order async completion, causing stale
database state to overwrite newer state. Add a request identifier (such as a
counter) that increments each time `setActiveDB()` is invoked, capture this
identifier in the async operation, and only update `this.databases.activeDB`
when the returned handle belongs to the most recent request. This ensures that
slower earlier requests do not overwrite more recent database switches.
In `@ios/Shared/RocketChat/Database.swift`:
- Around line 73-79: The sqlite3_open call assumes the SQLite/ subdirectory
already exists, but on fresh installs or upgrades this directory may not be
present, causing the operation to fail. Before calling sqlite3_open with the
path variable, create the sqliteDir directory using
FileManager.default.createDirectory with withIntermediateDirectories set to
true. Add appropriate error handling to ensure the directory creation succeeds
before attempting to open the database connection.
---
Outside diff comments:
In `@app/lib/database/driver/connection.ts`:
- Around line 176-201: The issue in the openDb function is that if either
applyOpenPragmas or migrate throws an error, the sqlite handle opened by
openDatabaseAsync remains unclosed and uncached, causing a resource leak. Wrap
the applyOpenPragmas and migrate operations in a try-catch block, and in the
catch block, close the sqlite handle by calling its appropriate close method
before re-throwing the error to ensure proper cleanup.
- Around line 71-85: The deriveServerDbName() function can generate the same
filename as the reserved DEFAULT_DB_NAME constant ('default.db'), which occurs
when processing server URLs like 'https://default'. This creates a collision
where app database traffic and global servers database traffic get routed to the
same file. Fix this by adding a collision detection and prevention mechanism in
deriveServerDbName(): after deriving the sanitized database name, check if it
matches DEFAULT_DB_NAME and if so, apply a differentiating suffix or alternative
naming strategy to ensure uniqueness. Apply the same fix to the other similar
logic mentioned at lines 210-220 to maintain consistency across all database
filename derivation.
---
Nitpick comments:
In `@app/lib/database/facade/decorators.ts`:
- Around line 119-125: The `get` method in the `@json` decorator is re-parsing
and re-sanitizing the value on every access instead of using the memoization
cache. Modify the getter to first check if a cached value exists in the model's
`_jsonDecoratorCache` using the `rawFieldName` as the key. If cached, return it
immediately. Otherwise, perform the existing parsing and sanitization logic
(parseJSON and sanitizer calls), store the result in `_jsonDecoratorCache` with
the `rawFieldName` key, and then return the sanitized value. This preserves the
memoized behavior and avoids redundant parsing and sanitization work on
subsequent accesses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81a6d09c-6e06-4b53-bf0e-5f2bd5d07766
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (109)
.eslintrc.jsapp/containers/Avatar/useAvatarETag.tsapp/containers/MessageComposer/MessageComposer.tsxapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/containers/MessageErrorActions.tsxapp/definitions/IEmoji.tsapp/definitions/ILoggedUser.tsapp/definitions/IMessage.tsapp/definitions/IPermission.tsapp/definitions/IRole.tsapp/definitions/IRoom.tsapp/definitions/IServer.tsapp/definitions/IServerHistory.tsapp/definitions/ISettings.tsapp/definitions/ISlashCommand.tsapp/definitions/ISubscription.tsapp/definitions/IThread.tsapp/definitions/IThreadMessage.tsapp/definitions/IUpload.tsapp/definitions/IUser.tsapp/lib/database/driver/__tests__/connection.test.tsapp/lib/database/driver/connection.tsapp/lib/database/facade/Collection.tsapp/lib/database/facade/Database.tsapp/lib/database/facade/Model.tsapp/lib/database/facade/Q.tsapp/lib/database/facade/Query.tsapp/lib/database/facade/__tests__/facade.test.tsapp/lib/database/facade/decorators.tsapp/lib/database/facade/index.tsapp/lib/database/facade/observe.tsapp/lib/database/facade/schema.tsapp/lib/database/facade/translate.tsapp/lib/database/facade/writer.tsapp/lib/database/index.tsapp/lib/database/interfaces.tsapp/lib/database/model/CustomEmoji.jsapp/lib/database/model/FrequentlyUsedEmoji.jsapp/lib/database/model/Message.jsapp/lib/database/model/Permission.jsapp/lib/database/model/Role.jsapp/lib/database/model/Room.jsapp/lib/database/model/ServersHistory.jsapp/lib/database/model/Setting.jsapp/lib/database/model/SlashCommand.jsapp/lib/database/model/Subscription.jsapp/lib/database/model/Thread.jsapp/lib/database/model/ThreadMessage.jsapp/lib/database/model/Upload.jsapp/lib/database/model/User.jsapp/lib/database/model/servers/Server.jsapp/lib/database/model/servers/User.jsapp/lib/database/schema/app.jsapp/lib/database/schema/servers.jsapp/lib/database/services/Subscription.tsapp/lib/database/tableMaps.tsapp/lib/encryption/encryption.tsapp/lib/hooks/useFrequentlyUsedEmoji.tsapp/lib/methods/AudioManager.tsapp/lib/methods/emojis.tsapp/lib/methods/getCustomEmojis.tsapp/lib/methods/getPermissions.tsapp/lib/methods/getRoles.tsapp/lib/methods/getSettings.tsapp/lib/methods/getSlashCommands.tsapp/lib/methods/getThreadName.tsapp/lib/methods/getUsersPresence.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/lib/methods/helpers/markMessagesRead.tsapp/lib/methods/loadThreadMessages.tsapp/lib/methods/logout.tsapp/lib/methods/search.tsapp/lib/methods/sendFileMessage/sendFileMessage.tsapp/lib/methods/sendFileMessage/utils.tsapp/lib/methods/sendMessage.tsapp/lib/methods/subscriptions/room.tsapp/lib/methods/subscriptions/rooms.tsapp/lib/methods/updateMessages.tsapp/lib/services/connect.tsapp/sagas/createChannel.jsapp/sagas/createDiscussion.jsapp/sagas/login.jsapp/sagas/messages.jsapp/sagas/rooms.jsapp/sagas/selectServer.tsapp/views/AddExistingChannelView/index.tsxapp/views/NewMessageView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/views/RoomActionsView/index.tsxapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/views/RoomMembersView/helpers.tsapp/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/views/RoomView/List/hooks/useMessages.tsapp/views/RoomView/UploadProgress.tsxapp/views/RoomView/index.tsxapp/views/RoomsListView/hooks/useSubscriptions.tsapp/views/SearchMessagesView/index.tsxapp/views/SelectServerView.tsxapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/views/ShareView/index.tsxapp/views/TeamChannelsView.tsxapp/views/ThreadMessagesView/index.tsxbabel.config.jsios/Shared/RocketChat/Database.swiftmetro.config.jspackage.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/definitions/ISettings.tsapp/definitions/IThread.tsapp/definitions/IPermission.tsapp/lib/database/model/CustomEmoji.jsapp/definitions/IUpload.tsapp/lib/database/model/Room.jsapp/definitions/IEmoji.tsapp/lib/database/interfaces.tsapp/lib/database/model/FrequentlyUsedEmoji.jsapp/lib/database/model/Role.jsapp/lib/database/model/servers/User.jsapp/lib/database/model/SlashCommand.jsapp/lib/database/model/Setting.jsapp/lib/database/model/Permission.jsapp/definitions/IThreadMessage.tsapp/lib/database/services/Subscription.tsapp/lib/methods/helpers/markMessagesRead.tsapp/lib/database/facade/writer.tsapp/lib/methods/getSlashCommands.tsapp/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/lib/methods/getThreadName.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/SelectServerView.tsxapp/definitions/IServerHistory.tsapp/lib/database/facade/schema.tsapp/sagas/createDiscussion.jsapp/lib/database/model/Subscription.jsapp/lib/database/schema/app.jsapp/lib/methods/getRoles.tsapp/definitions/IMessage.tsapp/lib/methods/emojis.tsapp/containers/Avatar/useAvatarETag.tsapp/lib/methods/getSettings.tsapp/sagas/messages.jsapp/views/AddExistingChannelView/index.tsxmetro.config.jsapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/lib/database/model/ServersHistory.jsapp/definitions/IRoom.tsapp/definitions/ILoggedUser.tsapp/views/RoomView/List/hooks/useMessages.tsapp/definitions/IRole.tsapp/definitions/ISubscription.tsapp/lib/database/model/servers/Server.jsapp/lib/methods/sendMessage.tsapp/lib/methods/AudioManager.tsapp/views/NewMessageView/index.tsxapp/containers/MessageErrorActions.tsxbabel.config.jsapp/definitions/IServer.tsapp/lib/database/model/ThreadMessage.jsapp/views/TeamChannelsView.tsxapp/lib/database/facade/index.tsapp/lib/database/model/User.jsapp/lib/hooks/useFrequentlyUsedEmoji.tsapp/definitions/IUser.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/lib/database/model/Upload.jsapp/sagas/rooms.jsapp/sagas/selectServer.tsapp/lib/methods/getUsersPresence.tsapp/views/RoomsListView/hooks/useSubscriptions.tsapp/lib/database/driver/__tests__/connection.test.tsapp/views/SearchMessagesView/index.tsxapp/lib/methods/updateMessages.tsapp/lib/methods/getCustomEmojis.tsapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/lib/methods/search.tsapp/lib/database/facade/Query.tsapp/definitions/ISlashCommand.tsapp/lib/database/facade/translate.tsapp/views/ThreadMessagesView/index.tsxapp/lib/database/model/Thread.jsapp/lib/methods/subscriptions/rooms.tsapp/lib/methods/sendFileMessage/sendFileMessage.tsapp/lib/methods/getPermissions.tsapp/lib/database/tableMaps.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/subscriptions/room.tsapp/sagas/createChannel.jsapp/lib/methods/loadThreadMessages.tsapp/lib/methods/sendFileMessage/utils.tsapp/views/RoomMembersView/helpers.tsapp/lib/database/model/Message.jsapp/sagas/login.jsapp/lib/methods/logout.tsapp/views/ShareView/index.tsxapp/lib/database/facade/Q.tsapp/lib/database/facade/Database.tsapp/lib/encryption/encryption.tsapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/lib/database/index.tsapp/lib/database/facade/decorators.tsapp/lib/services/connect.tsapp/lib/database/facade/observe.tsapp/views/RoomActionsView/index.tsxapp/lib/database/facade/Model.tsapp/lib/database/facade/Collection.tsapp/lib/database/driver/connection.tsapp/lib/database/facade/__tests__/facade.test.tsapp/views/RoomView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/lib/database/schema/servers.jsapp/views/RoomView/UploadProgress.tsxapp/containers/MessageComposer/MessageComposer.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/definitions/ISettings.tsapp/definitions/IThread.tsapp/definitions/IPermission.tsapp/definitions/IUpload.tsapp/definitions/IEmoji.tsapp/lib/database/interfaces.tsapp/definitions/IThreadMessage.tsapp/lib/database/services/Subscription.tsapp/lib/methods/helpers/markMessagesRead.tsapp/lib/database/facade/writer.tsapp/lib/methods/getSlashCommands.tsapp/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/lib/methods/getThreadName.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/SelectServerView.tsxapp/definitions/IServerHistory.tsapp/lib/database/facade/schema.tsapp/lib/methods/getRoles.tsapp/definitions/IMessage.tsapp/lib/methods/emojis.tsapp/containers/Avatar/useAvatarETag.tsapp/lib/methods/getSettings.tsapp/views/AddExistingChannelView/index.tsxapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/definitions/IRoom.tsapp/definitions/ILoggedUser.tsapp/views/RoomView/List/hooks/useMessages.tsapp/definitions/IRole.tsapp/definitions/ISubscription.tsapp/lib/methods/sendMessage.tsapp/lib/methods/AudioManager.tsapp/views/NewMessageView/index.tsxapp/containers/MessageErrorActions.tsxapp/definitions/IServer.tsapp/views/TeamChannelsView.tsxapp/lib/database/facade/index.tsapp/lib/hooks/useFrequentlyUsedEmoji.tsapp/definitions/IUser.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/sagas/selectServer.tsapp/lib/methods/getUsersPresence.tsapp/views/RoomsListView/hooks/useSubscriptions.tsapp/lib/database/driver/__tests__/connection.test.tsapp/views/SearchMessagesView/index.tsxapp/lib/methods/updateMessages.tsapp/lib/methods/getCustomEmojis.tsapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/lib/methods/search.tsapp/lib/database/facade/Query.tsapp/definitions/ISlashCommand.tsapp/lib/database/facade/translate.tsapp/views/ThreadMessagesView/index.tsxapp/lib/methods/subscriptions/rooms.tsapp/lib/methods/sendFileMessage/sendFileMessage.tsapp/lib/methods/getPermissions.tsapp/lib/database/tableMaps.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/subscriptions/room.tsapp/lib/methods/loadThreadMessages.tsapp/lib/methods/sendFileMessage/utils.tsapp/views/RoomMembersView/helpers.tsapp/lib/methods/logout.tsapp/views/ShareView/index.tsxapp/lib/database/facade/Q.tsapp/lib/database/facade/Database.tsapp/lib/encryption/encryption.tsapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/lib/database/index.tsapp/lib/database/facade/decorators.tsapp/lib/services/connect.tsapp/lib/database/facade/observe.tsapp/views/RoomActionsView/index.tsxapp/lib/database/facade/Model.tsapp/lib/database/facade/Collection.tsapp/lib/database/driver/connection.tsapp/lib/database/facade/__tests__/facade.test.tsapp/views/RoomView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/views/RoomView/UploadProgress.tsxapp/containers/MessageComposer/MessageComposer.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/definitions/ISettings.tsapp/definitions/IThread.tsapp/definitions/IPermission.tsapp/lib/database/model/CustomEmoji.jsapp/definitions/IUpload.tsapp/lib/database/model/Room.jsapp/definitions/IEmoji.tsapp/lib/database/interfaces.tsapp/lib/database/model/FrequentlyUsedEmoji.jsapp/lib/database/model/Role.jsapp/lib/database/model/servers/User.jsapp/lib/database/model/SlashCommand.jsapp/lib/database/model/Setting.jsapp/lib/database/model/Permission.jsapp/definitions/IThreadMessage.tsapp/lib/database/services/Subscription.tsapp/lib/methods/helpers/markMessagesRead.tsapp/lib/database/facade/writer.tsapp/lib/methods/getSlashCommands.tsapp/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/lib/methods/getThreadName.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/SelectServerView.tsxapp/definitions/IServerHistory.tsapp/lib/database/facade/schema.tsapp/sagas/createDiscussion.jsapp/lib/database/model/Subscription.jsapp/lib/database/schema/app.jsapp/lib/methods/getRoles.tsapp/definitions/IMessage.tsapp/lib/methods/emojis.tsapp/containers/Avatar/useAvatarETag.tsapp/lib/methods/getSettings.tsapp/sagas/messages.jsapp/views/AddExistingChannelView/index.tsxmetro.config.jsapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/lib/database/model/ServersHistory.jsapp/definitions/IRoom.tsapp/definitions/ILoggedUser.tsapp/views/RoomView/List/hooks/useMessages.tsapp/definitions/IRole.tsapp/definitions/ISubscription.tsapp/lib/database/model/servers/Server.jsapp/lib/methods/sendMessage.tsapp/lib/methods/AudioManager.tsapp/views/NewMessageView/index.tsxapp/containers/MessageErrorActions.tsxbabel.config.jsapp/definitions/IServer.tsapp/lib/database/model/ThreadMessage.jsapp/views/TeamChannelsView.tsxapp/lib/database/facade/index.tsapp/lib/database/model/User.jsapp/lib/hooks/useFrequentlyUsedEmoji.tsapp/definitions/IUser.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/lib/database/model/Upload.jsapp/sagas/rooms.jsapp/sagas/selectServer.tsapp/lib/methods/getUsersPresence.tsapp/views/RoomsListView/hooks/useSubscriptions.tsapp/lib/database/driver/__tests__/connection.test.tsapp/views/SearchMessagesView/index.tsxapp/lib/methods/updateMessages.tsapp/lib/methods/getCustomEmojis.tsapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/lib/methods/search.tsapp/lib/database/facade/Query.tsapp/definitions/ISlashCommand.tsapp/lib/database/facade/translate.tsapp/views/ThreadMessagesView/index.tsxapp/lib/database/model/Thread.jsapp/lib/methods/subscriptions/rooms.tsapp/lib/methods/sendFileMessage/sendFileMessage.tsapp/lib/methods/getPermissions.tsapp/lib/database/tableMaps.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/subscriptions/room.tsapp/sagas/createChannel.jspackage.jsonapp/lib/methods/loadThreadMessages.tsapp/lib/methods/sendFileMessage/utils.tsapp/views/RoomMembersView/helpers.tsapp/lib/database/model/Message.jsapp/sagas/login.jsapp/lib/methods/logout.tsapp/views/ShareView/index.tsxapp/lib/database/facade/Q.tsapp/lib/database/facade/Database.tsapp/lib/encryption/encryption.tsapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/lib/database/index.tsapp/lib/database/facade/decorators.tsapp/lib/services/connect.tsapp/lib/database/facade/observe.tsapp/views/RoomActionsView/index.tsxapp/lib/database/facade/Model.tsapp/lib/database/facade/Collection.tsapp/lib/database/driver/connection.tsapp/lib/database/facade/__tests__/facade.test.tsapp/views/RoomView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/lib/database/schema/servers.jsapp/views/RoomView/UploadProgress.tsxapp/containers/MessageComposer/MessageComposer.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/definitions/ISettings.tsapp/definitions/IThread.tsapp/definitions/IPermission.tsapp/lib/database/model/CustomEmoji.jsapp/definitions/IUpload.tsapp/lib/database/model/Room.jsapp/definitions/IEmoji.tsapp/lib/database/interfaces.tsapp/lib/database/model/FrequentlyUsedEmoji.jsapp/lib/database/model/Role.jsapp/lib/database/model/servers/User.jsapp/lib/database/model/SlashCommand.jsapp/lib/database/model/Setting.jsapp/lib/database/model/Permission.jsapp/definitions/IThreadMessage.tsapp/lib/database/services/Subscription.tsapp/lib/methods/helpers/markMessagesRead.tsapp/lib/database/facade/writer.tsapp/lib/methods/getSlashCommands.tsapp/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/lib/methods/getThreadName.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/SelectServerView.tsxapp/definitions/IServerHistory.tsapp/lib/database/facade/schema.tsapp/sagas/createDiscussion.jsapp/lib/database/model/Subscription.jsapp/lib/database/schema/app.jsapp/lib/methods/getRoles.tsapp/definitions/IMessage.tsapp/lib/methods/emojis.tsapp/containers/Avatar/useAvatarETag.tsapp/lib/methods/getSettings.tsapp/sagas/messages.jsapp/views/AddExistingChannelView/index.tsxmetro.config.jsapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/lib/database/model/ServersHistory.jsapp/definitions/IRoom.tsapp/definitions/ILoggedUser.tsapp/views/RoomView/List/hooks/useMessages.tsapp/definitions/IRole.tsapp/definitions/ISubscription.tsapp/lib/database/model/servers/Server.jsapp/lib/methods/sendMessage.tsapp/lib/methods/AudioManager.tsapp/views/NewMessageView/index.tsxapp/containers/MessageErrorActions.tsxbabel.config.jsapp/definitions/IServer.tsapp/lib/database/model/ThreadMessage.jsapp/views/TeamChannelsView.tsxapp/lib/database/facade/index.tsapp/lib/database/model/User.jsapp/lib/hooks/useFrequentlyUsedEmoji.tsapp/definitions/IUser.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/lib/database/model/Upload.jsapp/sagas/rooms.jsapp/sagas/selectServer.tsapp/lib/methods/getUsersPresence.tsapp/views/RoomsListView/hooks/useSubscriptions.tsapp/lib/database/driver/__tests__/connection.test.tsapp/views/SearchMessagesView/index.tsxapp/lib/methods/updateMessages.tsapp/lib/methods/getCustomEmojis.tsapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/lib/methods/search.tsapp/lib/database/facade/Query.tsapp/definitions/ISlashCommand.tsapp/lib/database/facade/translate.tsapp/views/ThreadMessagesView/index.tsxapp/lib/database/model/Thread.jsapp/lib/methods/subscriptions/rooms.tsapp/lib/methods/sendFileMessage/sendFileMessage.tsapp/lib/methods/getPermissions.tsapp/lib/database/tableMaps.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/subscriptions/room.tsapp/sagas/createChannel.jsapp/lib/methods/loadThreadMessages.tsapp/lib/methods/sendFileMessage/utils.tsapp/views/RoomMembersView/helpers.tsapp/lib/database/model/Message.jsapp/sagas/login.jsapp/lib/methods/logout.tsapp/views/ShareView/index.tsxapp/lib/database/facade/Q.tsapp/lib/database/facade/Database.tsapp/lib/encryption/encryption.tsapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/lib/database/index.tsapp/lib/database/facade/decorators.tsapp/lib/services/connect.tsapp/lib/database/facade/observe.tsapp/views/RoomActionsView/index.tsxapp/lib/database/facade/Model.tsapp/lib/database/facade/Collection.tsapp/lib/database/driver/connection.tsapp/lib/database/facade/__tests__/facade.test.tsapp/views/RoomView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/lib/database/schema/servers.jsapp/views/RoomView/UploadProgress.tsxapp/containers/MessageComposer/MessageComposer.tsx
app/definitions/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared TypeScript type definitions in 'app/definitions/' directory
Files:
app/definitions/ISettings.tsapp/definitions/IThread.tsapp/definitions/IPermission.tsapp/definitions/IUpload.tsapp/definitions/IEmoji.tsapp/definitions/IThreadMessage.tsapp/definitions/IServerHistory.tsapp/definitions/IMessage.tsapp/definitions/IRoom.tsapp/definitions/ILoggedUser.tsapp/definitions/IRole.tsapp/definitions/ISubscription.tsapp/definitions/IServer.tsapp/definitions/IUser.tsapp/definitions/ISlashCommand.ts
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in 'app/views/' directory
Files:
app/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/views/SelectServerView.tsxapp/views/AddExistingChannelView/index.tsxapp/views/RoomView/List/hooks/useMessages.tsapp/views/NewMessageView/index.tsxapp/views/TeamChannelsView.tsxapp/views/RoomsListView/hooks/useSubscriptions.tsapp/views/SearchMessagesView/index.tsxapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/views/ThreadMessagesView/index.tsxapp/views/RoomMembersView/helpers.tsapp/views/ShareView/index.tsxapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/views/RoomActionsView/index.tsxapp/views/RoomView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/views/RoomView/UploadProgress.tsx
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable UI components in 'app/containers/' directory
Files:
app/containers/MessageComposer/components/SendThreadToChannel.tsxapp/containers/Avatar/useAvatarETag.tsapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/containers/MessageErrorActions.tsxapp/containers/MessageComposer/MessageComposer.tsx
app/sagas/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place sagas in 'app/sagas/' directory for handling side effects
Files:
app/sagas/selectServer.ts
app/lib/encryption/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place end-to-end encryption implementation in 'app/lib/encryption/' directory using
@rocket.chat/mobile-crypto
Files:
app/lib/encryption/encryption.ts
app/lib/services/connect.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use 'app/lib/services/connect.ts' for server connection management
Files:
app/lib/services/connect.ts
🧠 Learnings (4)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/definitions/ISettings.tsapp/definitions/IThread.tsapp/definitions/IPermission.tsapp/definitions/IUpload.tsapp/definitions/IEmoji.tsapp/lib/database/interfaces.tsapp/definitions/IThreadMessage.tsapp/lib/database/services/Subscription.tsapp/lib/methods/helpers/markMessagesRead.tsapp/lib/database/facade/writer.tsapp/lib/methods/getSlashCommands.tsapp/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/lib/methods/getThreadName.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/SelectServerView.tsxapp/definitions/IServerHistory.tsapp/lib/database/facade/schema.tsapp/lib/methods/getRoles.tsapp/definitions/IMessage.tsapp/lib/methods/emojis.tsapp/containers/Avatar/useAvatarETag.tsapp/lib/methods/getSettings.tsapp/views/AddExistingChannelView/index.tsxapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/definitions/IRoom.tsapp/definitions/ILoggedUser.tsapp/views/RoomView/List/hooks/useMessages.tsapp/definitions/IRole.tsapp/definitions/ISubscription.tsapp/lib/methods/sendMessage.tsapp/lib/methods/AudioManager.tsapp/views/NewMessageView/index.tsxapp/containers/MessageErrorActions.tsxapp/definitions/IServer.tsapp/views/TeamChannelsView.tsxapp/lib/database/facade/index.tsapp/lib/hooks/useFrequentlyUsedEmoji.tsapp/definitions/IUser.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/sagas/selectServer.tsapp/lib/methods/getUsersPresence.tsapp/views/RoomsListView/hooks/useSubscriptions.tsapp/lib/database/driver/__tests__/connection.test.tsapp/views/SearchMessagesView/index.tsxapp/lib/methods/updateMessages.tsapp/lib/methods/getCustomEmojis.tsapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/lib/methods/search.tsapp/lib/database/facade/Query.tsapp/definitions/ISlashCommand.tsapp/lib/database/facade/translate.tsapp/views/ThreadMessagesView/index.tsxapp/lib/methods/subscriptions/rooms.tsapp/lib/methods/sendFileMessage/sendFileMessage.tsapp/lib/methods/getPermissions.tsapp/lib/database/tableMaps.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/subscriptions/room.tsapp/lib/methods/loadThreadMessages.tsapp/lib/methods/sendFileMessage/utils.tsapp/views/RoomMembersView/helpers.tsapp/lib/methods/logout.tsapp/views/ShareView/index.tsxapp/lib/database/facade/Q.tsapp/lib/database/facade/Database.tsapp/lib/encryption/encryption.tsapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/lib/database/index.tsapp/lib/database/facade/decorators.tsapp/lib/services/connect.tsapp/lib/database/facade/observe.tsapp/views/RoomActionsView/index.tsxapp/lib/database/facade/Model.tsapp/lib/database/facade/Collection.tsapp/lib/database/driver/connection.tsapp/lib/database/facade/__tests__/facade.test.tsapp/views/RoomView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/views/RoomView/UploadProgress.tsxapp/containers/MessageComposer/MessageComposer.tsx
📚 Learning: 2026-05-07T13:19:52.152Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7304
File: app/sagas/deepLinking.js:237-243
Timestamp: 2026-05-07T13:19:52.152Z
Learning: In this codebase’s Redux-Saga usage, remember that `yield put(action)` dispatches through the Redux store synchronously, and any saga(s) that synchronously react via action listeners (and synchronous `put` chains) will run to completion before the calling saga resumes at its next `yield`. As a result, within a single saga there is no scheduler interleaving between a `yield select(...)` and a subsequent `yield take(...)` at the next `yield` point, so a check-then-take pattern like `const state = yield select(...); if (state !== TARGET) { yield take(a => a.type === TARGET); }` is safe from TOCTOU races under the synchronous `put`/take model described above.
Applied to files:
app/sagas/createDiscussion.jsapp/sagas/messages.jsapp/sagas/rooms.jsapp/sagas/createChannel.jsapp/sagas/login.js
📚 Learning: 2026-02-05T13:55:00.974Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:00.974Z
Learning: In this repository, the dependency on react-native-image-crop-picker should reference the RocketChat fork (RocketChat/react-native-image-crop-picker) with explicit commit pins, not the upstream ivpusic/react-native-image-crop-picker. Update package.json dependencies (and any lockfile) to point to the fork URL and a specific commit, ensuring edge-to-edge Android fixes are included. This pattern should apply to all package.json files in the repo that declare this dependency.
Applied to files:
package.json
📚 Learning: 2026-05-07T17:47:14.516Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7303
File: package.json:5-5
Timestamp: 2026-05-07T17:47:14.516Z
Learning: When reviewing pnpm `packageManager` version pins in any `package.json` (e.g., `"packageManager": "pnpm@<version>"`), don’t rely solely on web-search results to determine whether a version exists. For very recently published versions, cross-check the target version against the official pnpm release page (https://github.com/pnpm/pnpm/releases) and the npm registry page for pnpm (https://www.npmjs.com/package/pnpm) before flagging the pinned version as non-existent.
Applied to files:
package.json
🔇 Additional comments (86)
.eslintrc.js (2)
1-18: LGTM!
188-201: LGTM!babel.config.js (1)
11-13: LGTM!metro.config.js (1)
9-9: LGTM!package.json (1)
135-135: Both dependencies are already at their latest stable versions.RxJS 7.8.2 is the latest stable release; newer versions (8.0.0-alpha.x) are pre-release. babel-plugin-inline-import@^3.0.0 already includes version 3.0.0, the latest stable release. No updates needed.
app/lib/database/tableMaps.ts (1)
65-107: LGTM!app/lib/database/facade/index.ts (1)
8-25: LGTM!app/lib/database/driver/__tests__/connection.test.ts (1)
45-79: LGTM!Also applies to: 190-199
app/lib/database/interfaces.ts (1)
1-72: LGTM!app/lib/database/facade/__tests__/facade.test.ts (1)
1-363: LGTM!app/lib/database/model/SlashCommand.js (1)
1-1: LGTM!app/lib/database/model/Upload.js (1)
1-1: LGTM!app/lib/methods/sendFileMessage/sendFileMessage.ts (1)
4-4: LGTM!app/lib/methods/sendFileMessage/utils.ts (1)
5-5: LGTM!app/views/SelectedUsersView/index.tsx (1)
9-9: LGTM!app/views/ShareListView/index.tsx (1)
10-10: LGTM!app/views/ShareView/index.tsx (1)
8-8: LGTM!app/lib/database/model/CustomEmoji.js (1)
1-1: LGTM!app/lib/database/model/Permission.js (1)
1-1: LGTM!app/containers/MessageComposer/hooks/useAutocomplete.ts (1)
3-3: LGTM!app/lib/database/services/Subscription.ts (1)
1-1: LGTM!app/lib/methods/emojis.ts (1)
1-1: LGTM!app/views/NewMessageView/index.tsx (1)
7-7: LGTM!app/views/NewServerView/hooks/useServersHistory.tsx (1)
3-3: LGTM!app/views/RoomActionsView/index.tsx (1)
10-10: LGTM!app/lib/database/model/Message.js (1)
1-1: LGTM!app/lib/database/model/Room.js (1)
1-1: LGTM!app/lib/database/model/Setting.js (1)
1-1: LGTM!app/lib/database/model/Subscription.js (1)
1-1: LGTM!app/lib/methods/AudioManager.ts (1)
3-3: LGTM!app/lib/methods/getThreadName.ts (1)
1-1: LGTM!app/lib/methods/handleMediaDownload.ts (1)
5-5: LGTM!app/lib/methods/subscriptions/rooms.ts (1)
4-5: LGTM!app/lib/database/model/Thread.js (1)
1-1: LGTM!app/lib/database/model/ThreadMessage.js (1)
1-1: LGTM!app/lib/database/model/User.js (1)
1-1: LGTM!app/lib/database/model/servers/Server.js (1)
1-1: LGTM!app/lib/database/model/servers/User.js (1)
1-1: LGTM!app/lib/encryption/encryption.ts (1)
18-18: LGTM!app/lib/methods/logout.ts (1)
3-3: LGTM!Also applies to: 56-57
app/lib/methods/search.ts (1)
1-1: LGTM!app/lib/database/schema/app.js (1)
1-1: LGTM!app/lib/database/schema/servers.js (1)
1-1: LGTM!app/definitions/IEmoji.ts (1)
1-1: LGTM!app/definitions/IMessage.ts (1)
3-3: LGTM!app/definitions/IPermission.ts (1)
1-1: LGTM!app/lib/methods/updateMessages.ts (1)
1-2: LGTM!app/views/RoomView/List/hooks/buildVisibleSystemTypesClause.ts (1)
1-1: LGTM!app/views/RoomView/List/hooks/useMessages.ts (1)
5-5: LGTM!app/definitions/ILoggedUser.ts (1)
1-1: LGTM!app/definitions/IServer.ts (1)
1-1: LGTM!app/definitions/ISubscription.ts (1)
1-1: LGTM!app/definitions/IUpload.ts (1)
1-1: LGTM!app/definitions/IUser.ts (1)
1-1: LGTM!app/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsx (1)
5-5: LGTM!app/views/SearchMessagesView/index.tsx (1)
8-8: LGTM!app/views/SelectServerView.tsx (1)
7-7: LGTM!app/definitions/IRole.ts (1)
1-1: LGTM!app/definitions/IRoom.ts (1)
1-1: LGTM!app/definitions/IServerHistory.ts (1)
1-1: LGTM!app/definitions/ISettings.ts (1)
1-1: LGTM!app/containers/Avatar/useAvatarETag.ts (1)
4-4: LGTM!app/containers/MessageComposer/MessageComposer.tsx (1)
6-6: LGTM!app/containers/MessageComposer/components/SendThreadToChannel.tsx (1)
6-6: LGTM!app/lib/hooks/useFrequentlyUsedEmoji.ts (1)
3-3: LGTM!app/definitions/ISlashCommand.ts (1)
1-1: LGTM!app/definitions/IThread.ts (1)
3-3: LGTM!app/definitions/IThreadMessage.ts (1)
1-1: LGTM!app/lib/methods/helpers/findSubscriptionsRooms.ts (1)
1-1: LGTM!app/lib/methods/loadThreadMessages.ts (1)
3-3: LGTM!app/lib/services/connect.ts (1)
4-4: LGTM!Also applies to: 51-58
app/sagas/rooms.js (1)
2-2: LGTM!app/sagas/selectServer.ts (1)
6-6: LGTM!app/sagas/createChannel.js (1)
2-2: LGTM!app/sagas/createDiscussion.js (1)
2-2: LGTM!app/sagas/login.js (1)
2-2: LGTM!app/sagas/messages.js (1)
2-2: LGTM!app/views/AddExistingChannelView/index.tsx (1)
6-6: LGTM!app/views/RoomMembersView/helpers.ts (1)
3-3: LGTM!app/views/TeamChannelsView.tsx (1)
6-6: LGTM!app/views/ThreadMessagesView/index.tsx (1)
7-7: LGTM!app/lib/database/facade/Q.ts (1)
1-151: LGTM!app/lib/database/facade/translate.ts (1)
1-109: LGTM!app/lib/database/facade/Query.ts (1)
1-62: LGTM!app/lib/database/facade/schema.ts (1)
12-125: LGTM!app/lib/database/facade/Model.ts (1)
16-164: LGTM!
Comment on lines
+242
to
+249
| return observeRow(_handle, relationTableName, () => { | ||
| const id = model._getRaw(columnName) as string | null; | ||
| if (!id) return null; | ||
| const col = model.collections.get(relationTableName); | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const rows = (col as any)._fetchSync({ id }); | ||
| return rows.length > 0 ? rows[0] : null; | ||
| }) as Observable<T | null>; |
Contributor
There was a problem hiding this comment.
Relation.observe() cannot emit null when relation is removed/deleted.
Line 242 uses observeRow, which suppresses null results. That means clearing the FK or deleting the related row won’t emit null, despite the declared Observable<T | null> contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/facade/decorators.ts` around lines 242 - 249, The
`observeRow` function used in the return statement suppresses null values, but
the declared return type `Observable<T | null>` indicates the observable should
emit null when relations are removed or deleted. Replace the use of `observeRow`
with an alternative observation mechanism that preserves null emissions,
ensuring that when the foreign key is cleared or the related row is deleted, the
observer receives a null value as expected by the contract defined in the return
type signature.
Comment on lines
+73
to
+82
| const emit = () => { | ||
| if (subscriber.closed) return; | ||
| const fresh = fetchFn(); | ||
| const shared = structuralShare(prevMap, fresh); | ||
| prevMap.clear(); | ||
| for (const row of shared) { | ||
| prevMap.set(row.id, row); | ||
| } | ||
| subscriber.next(shared); | ||
| }; |
Contributor
There was a problem hiding this comment.
Route fetch/emit failures through subscriber.error in all emit paths.
At Line 73-82, Line 108-112, and Line 143-157, fetchFn() exceptions are unguarded. When triggered by setTimeout, this can bypass RxJS error handling and surface as unhandled runtime exceptions.
Proposed fix
const emit = () => {
if (subscriber.closed) return;
- const fresh = fetchFn();
- const shared = structuralShare(prevMap, fresh);
- prevMap.clear();
- for (const row of shared) {
- prevMap.set(row.id, row);
- }
- subscriber.next(shared);
+ try {
+ const fresh = fetchFn();
+ const shared = structuralShare(prevMap, fresh);
+ prevMap.clear();
+ for (const row of shared) {
+ prevMap.set(row.id, row);
+ }
+ subscriber.next(shared);
+ } catch (e) {
+ subscriber.error(e);
+ }
}; const emit = () => {
if (subscriber.closed) return;
- const row = fetchFn();
- if (row !== null) subscriber.next(row);
+ try {
+ const row = fetchFn();
+ if (row !== null) subscriber.next(row);
+ } catch (e) {
+ subscriber.error(e);
+ }
}; const emit = (force = false) => {
if (subscriber.closed) return;
- const fresh = fetchFn();
- const shared = structuralShare(prevMap, fresh);
+ try {
+ const fresh = fetchFn();
+ const shared = structuralShare(prevMap, fresh);
- // Diff on watched columns only
- if (!force && sameByColumns(lastRows, shared, colSet)) return;
+ // Diff on watched columns only
+ if (!force && sameByColumns(lastRows, shared, colSet)) return;
- prevMap.clear();
- for (const row of shared) {
- prevMap.set(row.id, row);
- }
- lastRows = shared;
- subscriber.next(shared);
+ prevMap.clear();
+ for (const row of shared) {
+ prevMap.set(row.id, row);
+ }
+ lastRows = shared;
+ subscriber.next(shared);
+ } catch (e) {
+ subscriber.error(e);
+ }
};Also applies to: 108-112, 143-157
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/facade/observe.ts` around lines 73 - 82, Wrap the fetchFn()
calls in try-catch blocks throughout the file to properly handle exceptions
through RxJS error handling. In the emit function (starting at line 73), wrap
the const fresh = fetchFn() call in a try-catch block, and in the catch clause,
route the error to subscriber.error() before returning. Apply the same pattern
to the other two emit paths mentioned (around lines 108-112 and 143-157) to
ensure all fetchFn() exceptions are properly caught and passed through
subscriber.error() instead of propagating as unhandled runtime exceptions when
triggered by setTimeout callbacks.
Comment on lines
+60
to
+63
| setActiveDB = async (database = ''): Promise<void> => { | ||
| const handle = await openServerDb(database); | ||
| this.databases.activeDB = new Database(handle, appSchema, appTableMap, appModelMap) as unknown as TAppDatabase; | ||
| }; |
Contributor
There was a problem hiding this comment.
Guard setActiveDB() against out-of-order async completion.
If callers trigger server switches quickly, a slower earlier openServerDb() can resolve last and overwrite activeDB with stale state.
💡 Proposed fix
class DB {
databases: IDatabases = {};
+ private _activeSwitchToken = 0;
@@
setActiveDB = async (database = ''): Promise<void> => {
+ const token = ++this._activeSwitchToken;
const handle = await openServerDb(database);
+ if (token !== this._activeSwitchToken) {
+ return;
+ }
this.databases.activeDB = new Database(handle, appSchema, appTableMap, appModelMap) as unknown as TAppDatabase;
};
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/index.ts` around lines 60 - 63, The `setActiveDB()` method
has a race condition where multiple rapid calls can result in out-of-order async
completion, causing stale database state to overwrite newer state. Add a request
identifier (such as a counter) that increments each time `setActiveDB()` is
invoked, capture this identifier in the async operation, and only update
`this.databases.activeDB` when the returned handle belongs to the most recent
request. This ensures that slower earlier requests do not overwrite more recent
database switches.
Comment on lines
+73
to
79
| // New encrypted DBs live in a `SQLite/` subdirectory, isolated from the legacy | ||
| // plaintext WatermelonDB files at the container root. Must stay in lockstep with | ||
| // `DB_SUBDIRECTORY` / `resolveDbDirectory()` in the JS driver (connection.ts). | ||
| let sqliteDir = (groupRoot as NSString).appendingPathComponent("SQLite") | ||
| let path = (sqliteDir as NSString).appendingPathComponent(dbName) | ||
|
|
||
| guard sqlite3_open(path, &db) == SQLITE_OK else { |
Contributor
There was a problem hiding this comment.
Create the SQLite/ directory before calling sqlite3_open.
Line 79 can fail on fresh installs/upgrades if the subdirectory is missing, because this code assumes JS already created it.
💡 Proposed fix
- let sqliteDir = (groupRoot as NSString).appendingPathComponent("SQLite")
- let path = (sqliteDir as NSString).appendingPathComponent(dbName)
+ let sqliteDirUrl = URL(fileURLWithPath: groupRoot, isDirectory: true).appendingPathComponent("SQLite", isDirectory: true)
+ do {
+ try FileManager.default.createDirectory(at: sqliteDirUrl, withIntermediateDirectories: true)
+ } catch {
+ NSLog("[Database] Failed to create SQLite directory for %@: %@", dbName, String(describing: error))
+ return
+ }
+ let path = sqliteDirUrl.appendingPathComponent(dbName).path📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // New encrypted DBs live in a `SQLite/` subdirectory, isolated from the legacy | |
| // plaintext WatermelonDB files at the container root. Must stay in lockstep with | |
| // `DB_SUBDIRECTORY` / `resolveDbDirectory()` in the JS driver (connection.ts). | |
| let sqliteDir = (groupRoot as NSString).appendingPathComponent("SQLite") | |
| let path = (sqliteDir as NSString).appendingPathComponent(dbName) | |
| guard sqlite3_open(path, &db) == SQLITE_OK else { | |
| // New encrypted DBs live in a `SQLite/` subdirectory, isolated from the legacy | |
| // plaintext WatermelonDB files at the container root. Must stay in lockstep with | |
| // `DB_SUBDIRECTORY` / `resolveDbDirectory()` in the JS driver (connection.ts). | |
| let sqliteDirUrl = URL(fileURLWithPath: groupRoot, isDirectory: true).appendingPathComponent("SQLite", isDirectory: true) | |
| do { | |
| try FileManager.default.createDirectory(at: sqliteDirUrl, withIntermediateDirectories: true) | |
| } catch { | |
| NSLog("[Database] Failed to create SQLite directory for %@: %@", dbName, String(describing: error)) | |
| return | |
| } | |
| let path = sqliteDirUrl.appendingPathComponent(dbName).path | |
| guard sqlite3_open(path, &db) == SQLITE_OK else { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/Shared/RocketChat/Database.swift` around lines 73 - 79, The sqlite3_open
call assumes the SQLite/ subdirectory already exists, but on fresh installs or
upgrades this directory may not be present, causing the operation to fail.
Before calling sqlite3_open with the path variable, create the sqliteDir
directory using FileManager.default.createDirectory with
withIntermediateDirectories set to true. Add appropriate error handling to
ensure the directory creation succeeds before attempting to open the database
connection.
diegolmello
commented
Jun 19, 2026
diegolmello
left a comment
diegolmello
left a comment
Member
Author
There was a problem hiding this comment.
Review: facade cutover (110 files)
The semantic risk concentrates in the ~1k-line facade, not the mechanical import surgery — the call-site changes are clean and I found no missed @nozbe/watermelondb imports outside app/lib/database/. The ESLint ban rule is well-constructed (per-filetype overrides, correct excludedFiles, barrel import still allowed). The @children decorator bug is the one that can write stale data; the rest are observable-semantics drift vs WMDB.
Reviewed by an automated pass; treat inline comments as suggestions, not blockers.
| (childCollection as unknown as { schema: TableSchema }).schema, | ||
| (childCollection as unknown as { _drizzleTable: SQLiteTable })._drizzleTable, | ||
| (childCollection as unknown as { _handle: DbHandle })._handle, | ||
| Model |
Member
Author
There was a problem hiding this comment.
🟡 [medium] @children builds a Collection<Model> instead of reusing the registered child collection
Line 179–184 constructs new Collection(..., Model) using the hardcoded base Model class. Rows fetched through this collection come back as bare Model instances — @field/@date/@json getters are not wired, _raw is never updated via prepareUpdate, and batch() writes stale or missing columns.
The registered child collection already has the correct model constructor. Fix: query through the existing collection directly rather than constructing a new one:
const childCollection = model.collections.get(childTable) as ICollection;
// ...
const query = (childCollection as unknown as { query: (...args: unknown[]) => Query<Model> })
.query(Q.where(association.foreignKey, model.id));
cache[childTable] = query;
return query;Drop the new Collection(...) block entirely.
| const emit = () => { | ||
| if (subscriber.closed) return; | ||
| const row = fetchFn(); | ||
| if (row !== null) subscriber.next(row); |
Member
Author
There was a problem hiding this comment.
🟢 [low] observeRow omits subscriber.complete() on row deletion
When fetchFn() returns null (row deleted), only subscriber.next is skipped — subscriber.complete() is never called. WMDB's model.observe() completes the observable on model destruction. Callers using takeUntil, first(), or take(1) will not have their teardown run, causing memory leaks in long-lived components.
Fix:
const emit = () => {
if (subscriber.closed) return;
const row = fetchFn();
if (row !== null) {
subscriber.next(row);
} else {
subscriber.complete();
}
};| _dateCache: Map<number, Date> = new Map(); | ||
|
|
||
| /** Memoized JSON parse cache keyed by raw string value — mirrors WMDB @json memo=true. */ | ||
| _jsonDecoratorCache: Record<string, [unknown, unknown]> = {}; |
Member
Author
There was a problem hiding this comment.
🟢 [low] Dead field: _jsonDecoratorCache is declared but never used
The comment says it mirrors WMDB @json memo=true, but @json's getter in decorators.ts calls parseJSON on every access without consulting this cache. The field allocates an object per model instance for no benefit.
Either implement the memoization or drop the field. Hot fields like reactions, roles, and customFields are re-parsed on every render; memoizing by raw-string value would avoid repeated JSON.parse calls in list views:
get(this: AnyModel): unknown {
const model = this.asModel;
const rawValue = model._getRaw(rawFieldName) as string;
const cache = model._jsonDecoratorCache;
if (Object.prototype.hasOwnProperty.call(cache, rawFieldName)) {
const [cachedRaw, cachedVal] = cache[rawFieldName];
if (cachedRaw === rawValue) return cachedVal;
}
const parsed = sanitizer(parseJSON(rawValue), model);
cache[rawFieldName] = [rawValue, parsed];
return parsed;
}| function sameByColumns<T extends HasId>(prev: T[], next: T[], cols: Set<string>): boolean { | ||
| if (prev.length !== next.length) return false; | ||
| for (let i = 0; i < prev.length; i++) { | ||
| const a = rowData(prev[i]); |
Member
Author
There was a problem hiding this comment.
🟢 [low] sameByColumns diffs by array index, not by row id
When a row is inserted or deleted at any position other than the end, every subsequent row shifts by one index. prev[i] and next[i] are now different rows, so column comparison is meaningless — real updates on unchanged rows are suppressed (false negatives), and actual changes on moved rows may be missed or double-fired.
Fix: diff by id-keyed maps:
function sameByColumns<T extends HasId>(prev: T[], next: T[], cols: Set<string>): boolean {
if (prev.length !== next.length) return false;
const prevById = new Map(prev.map(r => [r.id, rowData(r)]));
for (const row of next) {
const a = prevById.get(row.id);
if (!a) return false;
const b = rowData(row);
for (const col of cols) {
if (a[col] !== b[col]) return false;
}
}
return true;
}
diegolmello
commented
Jun 19, 2026
diegolmello
left a comment
diegolmello
left a comment
Member
Author
There was a problem hiding this comment.
Structural review (NATIVE-1277). The 110-file call-site swap is mechanical and consistent — every @nozbe/watermelondb import now points at the facade barrel, and the connect.ts / logout.ts await refactors are necessary and correct. The ESLint ban on facade-internal imports is correctly structured (.js → no-restricted-imports, .ts/.tsx → the @typescript-eslint variant, facade dir exempted).
Bigger-picture concern: the facade is ~1,400 source lines across 10 files — a near-complete reimplementation of WatermelonDB's runtime (query DSL, SQL translation, reactive layer, all 6 decorators, schema builders, sanitizedRaw, writer queue), not a thin shim. That's a full ORM that has to stay correct for the entire burn-down window. Worth confirming the burn-down (NATIVE-1282) is scheduled tightly so this layer doesn't become permanent. Findings inline.
| (childCollection as unknown as { schema: TableSchema }).schema, | ||
| (childCollection as unknown as { _drizzleTable: SQLiteTable })._drizzleTable, | ||
| (childCollection as unknown as { _handle: DbHandle })._handle, | ||
| Model |
Member
Author
There was a problem hiding this comment.
bug: high — @children returns untyped base Model instances. The new Collection(...) here passes Model as the model class instead of the registered subclass, so children come back without their @field/@date/@json getters — reading typed props yields undefined. Database.get() already resolves the right class via _modelMap[table]; use the same lookup (db._modelMap[childTable] ?? Model).
| case 'take': | ||
| case 'skip': | ||
| return undefined; | ||
| case 'on': |
Member
Author
There was a problem hiding this comment.
bug: high — Q.on is exported but silently dropped. case 'on' returns undefined, so any query passing a Q.on(...) clause loses it → full table scan, no WHERE. Q.on is in the public barrel, so a future call-site migration that used it would break silently. Either remove Q.on from the barrel or throw here when an on clause is encountered.
|
|
||
| const sub = addDatabaseChangeListener(event => { | ||
| if (!event.databaseFilePath.endsWith(`/${handle.dbName}`)) return; | ||
| if (event.tableName !== tableName) return; |
Member
Author
There was a problem hiding this comment.
bug: med — observeRow ignores event.rowId and has no debounce. The filter checks db + table but not the row id, so every write to the table re-fires all model.observe() subscriptions in it; and unlike observeTable there's no ~16ms debounce, so batched writes cause N synchronous re-fetches. Add (!event.rowId || event.rowId === id) to the filter and the same debounce as observeTable.
| .run(); | ||
| } | ||
|
|
||
| model._pendingOp = null; |
Member
Author
There was a problem hiding this comment.
bug: med — _pendingOp cleared inside the transaction callback. If the Drizzle transaction throws (e.g. unique constraint), SQLite rolls the rows back but the _pendingOp = null already applied to earlier models in JS stands — those models look clean while their rows were rolled back, and can't be retried. Collect the models to clear in a list and null them only after the transaction returns successfully.
| const emit = () => { | ||
| if (subscriber.closed) return; | ||
| const row = fetchFn(); | ||
| if (row !== null) subscriber.next(row); |
Member
Author
There was a problem hiding this comment.
bug: low — observeRow never completes/errors on deletion. When the row goes away fetchFn() returns null and the Observable just stops emitting without complete()/error() (WMDB errors on deletion). Call sites using takeUntil or cleanup in subscribe can leak. Complete or error the subscriber when the row disappears.
| (db as any) | ||
| .update(drizzleTable) | ||
| .set(rest as never) | ||
| .where(eq((drizzleTable as never as Record<string, never>).id, id)) |
Member
Author
There was a problem hiding this comment.
slop: heavy — (db as any) ~10x and a brittle .id assumption. The expo-sqlite Drizzle driver is fully typed; the casts exist only because _handle.db drops the schema generic — type it as ExpoSQLiteDatabase<SchemaForKind<K>> and they disappear. The eq((drizzleTable as never as Record<string, never>).id, id) cast is the worst: it hard-codes a PK named id with zero type checking, silently wrong for any future table whose PK differs.
| _dateCache: Map<number, Date> = new Map(); | ||
|
|
||
| /** Memoized JSON parse cache keyed by raw string value — mirrors WMDB @json memo=true. */ | ||
| _jsonDecoratorCache: Record<string, [unknown, unknown]> = {}; |
Member
Author
There was a problem hiding this comment.
slop: moderate — _jsonDecoratorCache declared but never used. The @json getter re-parses + re-sanitizes on every access (allocation per render for attachment/reaction fields), and the unused field falsely implies memoization is active. Either implement the cache in the getter or delete the field.
| } | ||
| } else if (isValidNumber(value)) { | ||
| // number column, valid value | ||
| raw[key] = value || 0; |
Member
Author
There was a problem hiding this comment.
slop: moderate — dead value || 0. This branch is guarded by isValidNumber(value), so value is already a finite number; value || 0 equals value for every case. Reads as "zero out falsy" but never does. Just assign value.
| // Start with a raw where id will be set by the fn or sanitizedRaw | ||
| const raw = sanitizedRaw({}, this.schema); | ||
| const model = new this._ModelClass(this, raw); | ||
| model._pendingOp = 'create'; |
Member
Author
There was a problem hiding this comment.
slop: nit — prepareCreate sets _pendingOp before fn() here but after fn() in Model.prepareCreate (Model.ts:113). Not a live bug (fn doesn't read it) but the inconsistency will trip anyone auditing both paths. Pick one order.
| import { sanitizedRaw } from '@nozbe/watermelondb/RawRecord'; | ||
|
|
||
| import { type Model, Q } from '../database/facade'; | ||
| import { sanitizedRaw } from '../database/facade'; |
Member
Author
There was a problem hiding this comment.
slop: nit — two imports from the same facade barrel on consecutive lines. Merge into one import.
… into feat/native-1277-facade-cutover # Conflicts: # app/lib/database/driver/connection.ts
- children: query through registered collection instead of constructing bare Collection(Model)
- _pendingOp: clear after transaction commits, not inside the loop (prevents clearing on rollback)
- observeRow: add 16ms debounce matching observeTable
- _fetchSync id check: presence (=== undefined) not truthiness, so find('') rejects rather than full-scans
- writer enqueue: catch sync throws from fn so they reject the caller and don't stall the queue
- sameByColumns: diff by id map instead of array index so reordering is not a false positive
- Q.on: throw instead of silently returning undefined
- schema setRawCoerced: drop dead `|| 0` inside isValidNumber branch
- Collection.prepareCreate: tag _pendingOp after fn() to match Model.prepareCreate order
- updateMessages.ts: merge two consecutive imports from database/facade into one
- Database.ts: type db via SyncDb alias (BaseSQLiteDatabase<'sync',...>) to drop all (db as any) casts; resolve PK column via getTableColumns instead of unsafe cast
- Model: remove _jsonDecoratorCache (declared but never read)
Claude-Session: https://claude.ai/code/session_01TE9VsFTeXsXc8ssqeR8MJ7
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/database/facade/Database.ts`:
- Around line 9-10: Combine the duplicate imports on lines 9-10 that both import
from 'drizzle-orm/sqlite-core' into a single import statement, importing both
SQLiteTable and BaseSQLiteDatabase together in one line. Additionally, apply
Prettier formatting rules to the code at lines 93 and 101 by running the
Prettier formatter on the file to ensure consistent code style and resolve the
formatting violations that would block CI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 50bdeaa6-ee0d-42c3-8711-57a267eeeb66
📒 Files selected for processing (11)
app/lib/database/driver/__tests__/connection.test.tsapp/lib/database/facade/Collection.tsapp/lib/database/facade/Database.tsapp/lib/database/facade/Model.tsapp/lib/database/facade/decorators.tsapp/lib/database/facade/observe.tsapp/lib/database/facade/schema.tsapp/lib/database/facade/translate.tsapp/lib/database/facade/writer.tsapp/lib/methods/updateMessages.tsios/Shared/RocketChat/Database.swift
💤 Files with no reviewable changes (1)
- app/lib/database/facade/Model.ts
✅ Files skipped from review due to trivial changes (1)
- app/lib/methods/updateMessages.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- app/lib/database/facade/translate.ts
- app/lib/database/facade/schema.ts
- ios/Shared/RocketChat/Database.swift
- app/lib/database/facade/writer.ts
- app/lib/database/driver/tests/connection.test.ts
- app/lib/database/facade/Collection.ts
- app/lib/database/facade/observe.ts
- app/lib/database/facade/decorators.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/database/facade/Database.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/lib/database/facade/Database.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/lib/database/facade/Database.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/lib/database/facade/Database.ts
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/lib/database/facade/Database.ts
🪛 ESLint
app/lib/database/facade/Database.ts
[error] 9-9: '/home/jailuser/git/node_modules/drizzle-orm/sqlite-core/index.js' imported multiple times.
(import/no-duplicates)
[error] 10-10: '/home/jailuser/git/node_modules/drizzle-orm/sqlite-core/index.js' imported multiple times.
(import/no-duplicates)
[error] 93-93: Replace .values(model._raw·as·never) with ⏎↹↹↹↹↹↹.values(model._raw·as·never)⏎↹↹↹↹↹↹
(prettier/prettier)
[error] 101-101: Replace .where(eq(getTableColumns(drizzleTable).id,·model._raw.id)) with ⏎↹↹↹↹↹↹.where(eq(getTableColumns(drizzleTable).id,·model._raw.id))⏎↹↹↹↹↹↹
(prettier/prettier)
Comment on lines
+9
to
+10
| import type { SQLiteTable } from 'drizzle-orm/sqlite-core'; | ||
| import type { BaseSQLiteDatabase } from 'drizzle-orm/sqlite-core'; |
Contributor
There was a problem hiding this comment.
Fix lint/prettier violations that will block CI.
Line 9-10 has duplicate imports from the same module, and Line 93/Line 101 violate current Prettier formatting.
Suggested patch
-import type { SQLiteTable } from 'drizzle-orm/sqlite-core';
-import type { BaseSQLiteDatabase } from 'drizzle-orm/sqlite-core';
+import type { BaseSQLiteDatabase, SQLiteTable } from 'drizzle-orm/sqlite-core';
@@
- db.insert(drizzleTable).values(model._raw as never).run();
+ db.insert(drizzleTable)
+ .values(model._raw as never)
+ .run();
@@
- db.delete(drizzleTable).where(eq(getTableColumns(drizzleTable).id, model._raw.id)).run();
+ db.delete(drizzleTable)
+ .where(eq(getTableColumns(drizzleTable).id, model._raw.id))
+ .run();Also applies to: 93-93, 101-101
🧰 Tools
🪛 ESLint
[error] 9-9: '/home/jailuser/git/node_modules/drizzle-orm/sqlite-core/index.js' imported multiple times.
(import/no-duplicates)
[error] 10-10: '/home/jailuser/git/node_modules/drizzle-orm/sqlite-core/index.js' imported multiple times.
(import/no-duplicates)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/facade/Database.ts` around lines 9 - 10, Combine the
duplicate imports on lines 9-10 that both import from 'drizzle-orm/sqlite-core'
into a single import statement, importing both SQLiteTable and
BaseSQLiteDatabase together in one line. Additionally, apply Prettier formatting
rules to the code at lines 93 and 101 by running the Prettier formatter on the
file to ensure consistent code style and resolve the formatting violations that
would block CI.
Sources: Coding guidelines, Linters/SAST tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Replaces the app's direct
@nozbe/watermelondbusage with the WatermelonDB-shaped facade over the Drizzle/expo-sqlite driver, so the app runs on the SQLCipher-encrypted database while every call site keeps its existing shape. Adds the table/model maps the facade needs to build eachDatabase, wires the servers and app schemas through it, and adds an ESLint rule that bans new directwatermelondbimports.WatermelonDB stays installed only as the legacy migration reader; no JS runtime path uses it anymore.
Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1277
How to test or reproduce
TZ=UTC pnpm test— full suite passes (1632 tests).Data now lives in the encrypted database; behaviour is unchanged from the user's perspective.
Types of changes
Checklist
Further comments
Stacked on
feat/native-1276-native-readers; review/merge that first. Part of the WatermelonDB → encrypted SQLite cutover (NATIVE-1272).Summary by CodeRabbit
Refactor
Chores